Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(screensharing): picker performance and UI improvements #1003

Merged
merged 10 commits into from
Jan 31, 2025

Conversation

ShGKme
Copy link
Contributor

@ShGKme ShGKme commented Dec 23, 2024

☑️ Resolves

Performance issue 1: huge memory leak on quick cancel

If a user cancels screensharing before live previews are initialized, they won't be released in onBeforUnmount. The stream is not ready yet in onBeforUnmount, so we cannot release it yet.

Now checking that when stream is got, the video element to bind it to still exists and release otherwise.

How to test

  1. Open screenshare picker
  2. Quickly close it before it loads the list
  3. See logs in the main process console - stream is still active in main branch

Performance issue 2: sources list update is resource consuming

The list of sources is updated every second. It makes a short freeze every second. Then more sources are available - then more is the freeze and resource usage.

We may increase the interval to, e.g., 5 seconds.

But I'd consider an alternative - do it on visibility change. If a user goes from Talk Desktop to another window to open/close is and then goes back - it's refreshed.

The only case that is not covered here, if some window is closed on it's own. But the user just won't be able to share it in the end.

Performance issue 3: live preview is resource consuming

Live previews for many screens and windows may have a huge performance impact.

Making live preview optional.

Other changes

  • UI improvements
  • Refactoring
    • Move to a module
    • Migrate to TS

🖼️ Screenshots

🏚️ Before

image

🏡 After

image

Single source available

image

Only screens (no windows)

image

Wayland emulation

image

Video

screensharing.mp4

@ShGKme ShGKme added bug Something isn't working 2. developing labels Dec 23, 2024
@ShGKme ShGKme self-assigned this Dec 23, 2024
@ShGKme ShGKme force-pushed the fix/screensharing-performance branch from 7239513 to 3b4a827 Compare January 19, 2025 22:57
Signed-off-by: Grigorii K. Shartsev <me@shgk.me>
Signed-off-by: Grigorii K. Shartsev <me@shgk.me>
Signed-off-by: Grigorii K. Shartsev <me@shgk.me>
@ShGKme ShGKme force-pushed the fix/screensharing-performance branch from 3b4a827 to fd588af Compare January 31, 2025 12:30
@ShGKme ShGKme added this to the 🪴 Next Release milestone Jan 31, 2025
@ShGKme ShGKme force-pushed the fix/screensharing-performance branch from fd588af to 8281c85 Compare January 31, 2025 12:54
@ShGKme ShGKme force-pushed the fix/screensharing-performance branch from 8281c85 to d75c5a2 Compare January 31, 2025 13:21
@ShGKme ShGKme marked this pull request as ready for review January 31, 2025 13:21
Copy link
Contributor

@Antreesy Antreesy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested on Windows and WSL/Wayland

@ShGKme ShGKme force-pushed the fix/screensharing-performance branch from d75c5a2 to 4566538 Compare January 31, 2025 15:37
@ShGKme ShGKme changed the title fix(screensharing): performance and ux improvements fix(screensharing): picker performance and UI improvements Jan 31, 2025
Signed-off-by: Grigorii K. Shartsev <me@shgk.me>
Signed-off-by: Grigorii K. Shartsev <me@shgk.me>
Signed-off-by: Grigorii K. Shartsev <me@shgk.me>
Signed-off-by: Grigorii K. Shartsev <me@shgk.me>
Signed-off-by: Grigorii K. Shartsev <me@shgk.me>
Signed-off-by: Grigorii K. Shartsev <me@shgk.me>
Signed-off-by: Grigorii K. Shartsev <me@shgk.me>
@ShGKme ShGKme force-pushed the fix/screensharing-performance branch from 4566538 to 9659757 Compare January 31, 2025 19:53
@ShGKme
Copy link
Contributor Author

ShGKme commented Jan 31, 2025

Applied suggestions from review.

Replaces space_space with tab in all the files

@ShGKme ShGKme enabled auto-merge January 31, 2025 19:54
@ShGKme ShGKme merged commit f566876 into main Jan 31, 2025
10 checks passed
@ShGKme ShGKme deleted the fix/screensharing-performance branch January 31, 2025 19:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Nextcloud Talk Meeting Screen Sharing Issues
3 participants